Post author selector: try downshift #7478

adamsilverstein · 2018-06-22T13:35:12Z

Description

This PR adds a selector implementation built with downshift, an MIT licensed downshift from PayPal which aims to provide "Primitives to build simple, flexible, WAI-ARIA compliant React autocomplete/dropdown/select/combobox components"

Addresses #7384

How has this been tested?

Tried with small number and large number of users, added with wp user generate --role=editor --count=12000
Tested using only keyboard to interact.
Tested with voiceover, checking results announcements

Screenshots

Types of changes

include downshift, replacing author select
grab 100 users by default from REST API,

Questions / Todo

no special handling for smaller numbers of users, should we add that? eg use regular select element when < 50 users?
aria strings should be passed thru our i18n function

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.

adamsilverstein · 2018-06-22T13:35:48Z

@afercia appreciate a quick review of this approach as well when you have a moment!

youknowriad

Thanks for trying to move this forward. Might be good to get some a11y insights on Downshift.

youknowriad · 2018-06-27T08:31:35Z

editor/components/post-author/index.js


 /**
 * Internal dependencies
 */
 import PostAuthorCheck from './check';

+/**
+ * External dependencies
+ */


In general the external dependencies are defined above all other deps, but this is not an official guideline :)

youknowriad · 2018-06-27T08:31:49Z

editor/components/post-author/index.js

+ * External dependencies
+ */
+import Downshift from 'downshift';
+import { debounce } from 'underscore';


Should we use lodash instead?

oops, right :)

youknowriad · 2018-06-27T08:37:03Z

editor/components/post-author/index.js

+		}
+		const payload = '?search=' + encodeURIComponent( query );
+		this.setState( { searching: true } );
+		apiRequest( { path: '/wp/v2/users' + payload } ).done( ( results ) => {


Should we leverage the data module instead? We already have a users reducer their which I believe should be refactored to be an entity and build support to searching/pagination into the entities. related #6395

afercia · 2018-06-27T09:04:03Z

@adamsilverstein good morning, thanks for the ping. Looking just at the a11y side, I'm not particularly happy with what downshift does. Their implementation is basically wrong, especially with regards to aria-selected. It's not supposed to work that way.

When an option in the list gets highlighted, it should also get aria-selected="true":
https://www.w3.org/TR/wai-aria-practices/#combobox

In a combobox with a listbox, grid, or tree popup, when a suggested value is visually indicated as the currently selected value, the option, gridcell, row, or treeitem containing that value has aria-selected set to true.

Instead, they switch it to true only when it's the actual selected value. This is wrong. A correct usage of aria-selected is required to make the combobox work consistently across browsers/screen readers combinations. For example, the only reason why the highlighted option is announced with Safari + VoiceOver is because downshift uses an aria-live region to announce the option. Without that, Safari + VoiceOver wouldn't announce anything. However, a combobox is not supposed to use an aria-live region at all. With other browsers/screen readers combinations, the combobox works natively and the options get announced plus the change in the aria-live region gets announced. This makes the highlighted option be announced twice:

There are other minor quirks, for example:

the input content sometimes disappears when tabbing
the number of results gets announced multiple times (see screenshot above), seems it gets a first set of results and then a second set and so on (may depend on slow response maybe?)
other aria attributes that could be improved. Overall, from an a11y perspective, the Gutenberg auto-completers work better.

Aside: the component crashes in IE 11 because it uses includes():

Info for the ones willing to test downshift on their demo page: it doesn't work with Safari + VoiveOver because it's within an iframe; you have to access the iframe directly.

karmatosed · 2018-07-02T18:56:18Z

[ edit ] Re-uploading as something weird happened to my images - sorry for second post on this.

I have a suggested design, firstly lets have at the top as this is a common use. I would like to not have the input next to 'author' as we could have very probably cases of long author names. Let's have it as a section, this also allows it to have multiples in future.

The design itself takes a lot of inspiration from links and should work.

adamsilverstein · 2018-07-02T22:31:31Z

Looking just at the a11y side, I'm not particularly happy with what downshift does. Their implementation is basically wrong, especially with regards to aria-selected. It's not supposed to work that way.

@afercia :( Maybe I should double back on my autocompleter based implementation. Could you try this PR as well? #7385

adamsilverstein · 2018-07-02T22:34:32Z

@karmatosed Thanks for the design, that is great and I like how it feels to give the field more room. question: when there is already a user selected and i click in the box, should the field clear/show the placeholder?

afercia · 2018-07-03T15:39:24Z

Just noting the design of the field is one more case where the placeholder is used as replacement for a label. We've warned several times this is far from ideal for accessibility and goes against the HTML spec.

adamsilverstein · 2018-07-03T15:48:37Z

Just noting the design of the field is one more case where the placeholder is used as replacement for a label. We've warned several times this is far from ideal for accessibility and goes against the HTML spec.

Can we still have a label instead of a placeholder then, say something like Type to select user: maybe on a line above the selector so there is still room. This does seem to duplicate some information, is there a way to add a label that doesn't display anything but still gets read correctly?

afercia · 2018-07-03T15:56:00Z

We're using a lot of labels visually hidden with screen-reader-text. The real point is the placeholder shouldn't be used that way ;)

adamsilverstein · 2018-07-03T16:17:06Z

Got it, thanks for clarifying.

karmatosed · 2018-07-13T17:54:49Z

when there is already a user selected and i click in the box, should the field clear/show the placeholder?

Yes it should clear.

aduth · 2018-09-13T19:29:21Z

Is it likely this will be resumed, or can it be closed?

adamsilverstein · 2018-09-22T22:38:51Z

Is it likely this will be resumed, or can it be closed?

Closing this. I'll add any further work on #7385

Adam Silverstein added 6 commits June 21, 2018 13:59

Downshift based author selector, first pass

7213b6b

add searching

dec6f46

add a search active spinner

bcb97de

fixes for eslint

b898830

Only search authors if dropdown open, prevents search after selection

df22448

refinements for voiceover

9dcc5ad

adamsilverstein added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Design Needs design efforts. labels Jun 22, 2018

adamsilverstein mentioned this pull request Jun 22, 2018

Post author dropdown: add accessible search with debounced api request #7384

Closed

tofumatt requested review from afercia and a team June 26, 2018 17:17

youknowriad reviewed Jun 27, 2018

View reviewed changes

karmatosed added Needs Design Feedback Needs general design feedback. and removed Needs Design Needs design efforts. labels Jul 2, 2018

talldan self-requested a review July 9, 2018 04:22

karmatosed removed the Needs Design Feedback Needs general design feedback. label Jul 15, 2018

aduth added the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Sep 13, 2018

adamsilverstein closed this Sep 22, 2018

youknowriad deleted the feature/downshift-select branch September 24, 2018 06:55

adamsilverstein mentioned this pull request Jul 26, 2019

Parent Page Selector: use an accessible autocomplete/search component #16666

Closed

5 tasks

adamsilverstein mentioned this pull request Nov 3, 2019

Components: Add a WAI-ARIA compliant custom select. #17926

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Post author selector: try downshift #7478

Post author selector: try downshift #7478

adamsilverstein commented Jun 22, 2018 •

edited

Loading

adamsilverstein commented Jun 22, 2018

youknowriad left a comment

youknowriad Jun 27, 2018

youknowriad Jun 27, 2018

adamsilverstein Jul 2, 2018

youknowriad Jun 27, 2018

afercia commented Jun 27, 2018

karmatosed commented Jul 2, 2018 •

edited

Loading

adamsilverstein commented Jul 2, 2018

adamsilverstein commented Jul 2, 2018

afercia commented Jul 3, 2018

adamsilverstein commented Jul 3, 2018

afercia commented Jul 3, 2018

adamsilverstein commented Jul 3, 2018

karmatosed commented Jul 13, 2018

aduth commented Sep 13, 2018

adamsilverstein commented Sep 22, 2018

Post author selector: try downshift #7478

Post author selector: try downshift #7478

Conversation

adamsilverstein commented Jun 22, 2018 • edited Loading

Description

How has this been tested?

Screenshots

Types of changes

Questions / Todo

Checklist:

adamsilverstein commented Jun 22, 2018

youknowriad left a comment

Choose a reason for hiding this comment

youknowriad Jun 27, 2018

Choose a reason for hiding this comment

youknowriad Jun 27, 2018

Choose a reason for hiding this comment

adamsilverstein Jul 2, 2018

Choose a reason for hiding this comment

youknowriad Jun 27, 2018

Choose a reason for hiding this comment

afercia commented Jun 27, 2018

karmatosed commented Jul 2, 2018 • edited Loading

adamsilverstein commented Jul 2, 2018

adamsilverstein commented Jul 2, 2018

afercia commented Jul 3, 2018

adamsilverstein commented Jul 3, 2018

afercia commented Jul 3, 2018

adamsilverstein commented Jul 3, 2018

karmatosed commented Jul 13, 2018

aduth commented Sep 13, 2018

adamsilverstein commented Sep 22, 2018

adamsilverstein commented Jun 22, 2018 •

edited

Loading

karmatosed commented Jul 2, 2018 •

edited

Loading